Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catalog protocols #2250

Merged
merged 3 commits into from
Aug 2, 2016
Merged

Conversation

tiennou
Copy link
Member

@tiennou tiennou commented Jul 29, 2016

A rejig of #2246, with some tweaks to check_indent because it didn't like documentation comments and staged changes.

@tiennou tiennou closed this Jul 29, 2016
@tiennou tiennou reopened this Jul 29, 2016
@tiennou tiennou force-pushed the catalog/object-source-protocol branch from de692e2 to 1ee8a15 Compare August 1, 2016 22:03
@skurfer
Copy link
Member

skurfer commented Aug 2, 2016

I guess if we can live with the deprecation warnings, this doesn’t actually break anything, right? I’ll test it out for a bit.

@tiennou
Copy link
Member Author

tiennou commented Aug 2, 2016

It shouldn't. I just noticed I missed adding @optional at the top of QSObjectHandler, so I'll fix that.

@pjrobertson
Copy link
Member

I like this! Rob - if you're happy with what you've tested so far, I say go ahead with the merge

Aside: Wow, after typing "I like this!" I got a kind of spooky feeling... it reminded me of the commit message @tiennou made... 2 and a half years ago: 2719535 every time you force pushed that branch (and it was probably about 20 times!) I got an email who's content was just "@pjrobertson likes those"

Scary!

@skurfer
Copy link
Member

skurfer commented Aug 2, 2016

Between this and the task clean-up, I’m staring at a wall of deprecation warnings we’ve imposed on ourselves. 😄 I know it’s worth it, but I don’t want them to get too out of control.

It looks like you’ve taken on a lot of them in #2257. After merging this, I’ll see about getting currentEntry calls out of the core app (unless you have a branch for that in progress).

@skurfer skurfer merged commit 8a38b4d into quicksilver:master Aug 2, 2016
skurfer added a commit that referenced this pull request Aug 2, 2016
@tiennou
Copy link
Member Author

tiennou commented Aug 2, 2016

Yeah, right now I'm running tiennou/develop which is a heck of a monster-merge-thing, and it seems to work* mostly. I'm looking at 93 warnings, and most of them come from NIBs (26) and SUPlainInstallerInternals (14).

*There are some catalog problems because of unfinished librarian-cleanup being in, but it fixes itself after a rescan.

The weird thing is that somehow Xcode doesn't show those warnings here yet (they magically appeared when working with a plugin). I'll see what not using my Workspace does btw...

@tiennou
Copy link
Member Author

tiennou commented Aug 2, 2016

FYI there's a deprecated/breaking on top of #2257 that kills most of the process-related ones.

@tiennou tiennou deleted the catalog/object-source-protocol branch August 2, 2016 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants